Skip to content

test: the matrix must reconcile registered suites against accounted ones (#916) - #922

Merged
jdatcmd merged 8 commits into
mainfrom
feat/916-reconcile-accounting
Sep 10, 2026
Merged

test: the matrix must reconcile registered suites against accounted ones (#916)#922
jdatcmd merged 8 commits into
mainfrom
feat/916-reconcile-accounting

Conversation

@jdatcmd

@jdatcmd jdatcmd commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator

Closes #916 — phase 2 of #858.

What was wrong

run_all_versions.sh printed suites that ran: N of M and never checked it. Two different failures hid behind that line.

The arithmetic nobody did. ran + skipped + incomplete is printed beside M and never compared with it.

The one that is live today. pgc_classify_suite_rc maps rc=0 to PASS with no further question, and ten of the 251 registered suites exit 0 having never called pgc_summary. They assert things in their own way — concurrency prints its own failure and exits non-zero — but the harness cannot count their checks, and nothing says so. They are counted among the suites that "ran", which is the exact overcount #447 added that line to stop, one level further down.

Why this is not a count

A count cannot close it. Two errors of opposite sign cancel, and an exempt list maintained by hand makes the count agree by construction — the check then measures the list rather than the run.

A total derived from the collect loop is worse: that loop visits every registered name, so any sum over it is an identity rather than a measurement.

So membership is derived from a property each suite carries, and two readings taken from different places are reconciled as sets, in both directions:

reading source
declared the suite's own text calls pgc_summary
observed the suite's log carries the accounting: line pgc_summary prints before every one of its four exit paths

Neither is a number and neither is hand-maintained. A suite that stops calling pgc_summary moves between the sets on its own. The two directions catch opposite mistakes:

  • declared but never accounted — the suite died before reaching its summary. Today that reads PASS whenever the shell happened to exit 0.
  • accounted but never declared — the reading of the source is stale. This is the failure a hand-maintained exempt list can never report.

checks run: would not have worked as the marker: bench_guards, docs_style and pg_upgrade print their own. accounting: is produced by pgc_summary and by nothing else in the tree.

The third term, recorded rather than inferred

PGC_SKIP_TIMING=1 is set on every CI run and drops four suites. They declare accounting and correctly produce none, because nothing executed them. Without a term for that the check goes red for the one reason that is not a defect, and a check that cries wolf on every run is one nobody reads.

The driver records that decision in the branch that makes it, beside the log that branch forges. Inferring it from the forged log would mean trusting a marker the driver wrote on the suite's behalf, which is the kind of claim this whole check exists to stop. A suite appearing in both that record and the observed set is reported as its own distinct fault, since the union would otherwise absorb it.

Removal proofs

Each measured, not argued.

mutation what reddens
strip the comment-stripping from the declaration reader a comment mentioning pgc_summary is not a declaration
delete the record from the skip branch the skip branch records the suite it did not dispatch
drop the sort before comm the identity catches comm reading unsorted input — 20/20 trials
compute inputs from the buckets instead of the files nothing reddens

That last row is the point of the fourth arm. inputs == sum(buckets) is printed beside every reconciliation per the house rule, but a derived total makes it P + D + O == P + D + O — true for any values. inputs is therefore counted from the two files by a separate route, and both harnesses say so out loud and pin the fault it actually guards.

The bug this suite caught in its own implementation

The declaration reader's first version piped sed into grep -q. This suite runs under set -o pipefail. grep -q exits the moment it matches, closing the pipe while sed is still writing; sed takes EPIPE and exits non-zero, and pipefail reports the pipeline as failed even though grep matched. The reader answered no for a suite that plainly calls pgc_summary.

It is a race, so it reproduces on long files and not short ones. It passed every fixture in this PR and failed only on the real population, naming the two longest suites — analyze_function and hilbert_curve — and it showed up as declares accounting=237 where an independent reading said 239. Selftest 040 carries the same story from #473 and #476, where it named different innocent suites on every run.

The fix is grep -c, which reads to EOF. A regression arm in both harnesses pins it with a 40,000-line fixture, and shows the grep -q shape still gets it wrong there while agreeing on a short file — which is why it survived review the first time.

Evidence

selftest   exit=0   FAILs=0   checks run: 492   accounting: 492 passed + 0 failed + 0 unrunnable = 492
           registered=251 | declares accounting=239, does not=12 | sum=251

pytest     test_suite_accounting.py: 8 passed
           whole corpus: 121 passed, 35 errors -- all /usr/local/pg18a/bin/pg_config
           absent on this host, identical on main

shellcheck -S error -s bash test/*.sh test/selftest/*.sh   rc=0
docs_style                                                  PASSED (9 checks)

Both harnesses, one implementation: the pytest half drives the shell functions out of run_all_versions.sh rather than reimplementing them, because a Python twin would agree with itself.

TESTS.md gains section 14; sections 14-16 renumbered to 15-17, with the table of contents verified against the headers programmatically (contiguous 1..17, ToC == headers) rather than by reading — #910's orphaned heading is the reason.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK

jdatcmd and others added 2 commits September 9, 2026 20:21
…nes (#916)

`run_all_versions.sh` printed `suites that ran: N of M` and never checked it, and
ten registered suites exit 0 having never counted a check. `pgc_classify_suite_rc`
maps rc=0 to PASS with no further question, so those ten are counted among the
suites that "ran" -- the overcount #447 added that line to stop, one level further
down.

A count cannot close this. Two errors of opposite sign cancel, and an exempt list
maintained by hand makes the count agree by construction: the check then measures
the list rather than the run. A total derived from the collect loop is worse still,
because that loop visits every registered name and any sum over it is an identity.

So membership is derived from a property each suite carries, and two readings taken
from different places are reconciled as SETS, in both directions:

  declared  the suite's own text calls pgc_summary
  observed  the suite's log carries the `accounting:` line pgc_summary prints
            before every one of its four exit paths

Neither is a number and neither is hand-maintained. A suite that stops calling
pgc_summary moves between the sets on its own, and the two directions catch
opposite mistakes: declared-but-not-accounted is a suite that died before it could
account, and accounted-but-not-declared is a stale reading of the source.

A third term is the driver's own record of suites it chose not to dispatch.
PGC_SKIP_TIMING drops four on every CI run; they declare accounting and correctly
produce none. The record is written by the branch that makes the decision, not
inferred from the log that branch forges, and a suite in both that record and the
observed set is reported as its own distinct fault.

`checks run: 251` is not what discriminates: bench_guards, docs_style and
pg_upgrade print their own. `accounting:` is produced by pgc_summary and by nothing
else in the tree.

Removal proofs, each measured rather than argued:

  strip the comment-stripping from the declaration reader
      -> "a comment mentioning pgc_summary is not a declaration" reddens
  delete the record from the skip branch
      -> "the skip branch records the suite it did not dispatch" reddens
  drop the sort before comm
      -> "the identity catches comm reading unsorted input" reddens, 20/20
  compute inputs FROM the buckets instead of the files
      -> NOTHING reddens, which is why inputs is counted by a separate route
         and why that is said out loud in both harnesses

The declaration reader's first version piped sed into `grep -q`, and this suite runs
under `set -o pipefail`. grep -q exits the moment it matches, sed takes EPIPE, and
pipefail reports the pipeline as failed even though grep matched -- so a suite that
plainly calls pgc_summary read as not declaring accounting. It is a race, so it
passed every fixture and failed only on the real population, naming the two longest
suites, analyze_function and hilbert_curve. Selftest 040 carries the same story from
#473 and #476. The fix is grep -c, which reads to EOF; a regression arm in both
harnesses pins it with a 40,000-line fixture and shows the grep -q shape still gets
it wrong there while agreeing on a short file.

Both harnesses, one implementation: the pytest half drives the shell functions out
of run_all_versions.sh rather than reimplementing them, because a Python twin would
agree with itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
…s the shell (#916)

Both from OffgridwithJD's review of #922, and both are real.

AN ABSENT FILE IS NOT AN EXEMPT SUITE. pgc_suite_declares_accounting returned
"no" for a file that does not exist as readily as for one that does not call
pgc_summary. A registered suite whose .sh had vanished was therefore classified
exempt, and the reconciliation read clean -- a suite disappearing from the matrix,
inside the check whose whole subject is suites going missing from the accounting.

It now answers "absent", the runner counts those and fails the major, and the
real-population block in selftest 390 carries three buckets rather than folding
absent back into exempt. Measured today: 251 registered, 0 absent.

THE COMMENT STRIPPER NOW FOLLOWS THE SHELL'S RULE. `sed 's/#.*$//'` strips from
ANY hash, so a hash inside a quoted string earlier on the line would hide a
pgc_summary call after it. It now strips only a hash at line start or after
whitespace, which is what the shell treats as starting a comment.

Measured rather than assumed, over all 251 registered suites: three carry a line
holding both a hash and pgc_summary -- analyze_function, hilbert_curve and
projections -- and in every one the hash starts the line, so no suite was misread
either way. The partition is 239/12 before and after.

The shell rule still does not cover a hash after whitespace INSIDE a quoted
string, so that residual is pinned by an arm over the corpus rather than left to
be rediscovered: no registered suite may have a hash before a pgc_summary call on
the same line.

New arms in both harnesses: absent is distinguishable from exempt, a hash inside
a word does not hide the call after it, a trailing comment does not either, an
indented comment is still a comment, and every registered suite has a file.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
jdatcmd and others added 2 commits September 9, 2026 21:02
…he twelve are (#916)

All from OffgridwithJD's review, including two corrections to claims I had
already pushed.

THE CORPUS ARM I ADDED LAST COMMIT WAS INVERTED. It required a non-whitespace
character before the hash, which is a hash inside a WORD -- the shape the
stripper handles correctly -- so it flagged the safe case and was blind to the
dangerous one it was named for. Reproduced here with their control:

    psql -c "SELECT 1 # note"; pgc_summary    reader says NO, arm did not flag
    X=a#b; pgc_summary                        reader says YES, arm FLAGGED

The arm no longer restates the hazard as a second pattern. It compares the
reader's INPUT with its OUTPUT: count the call in the raw file, count it in the
stripped text, and a lower stripped count means the stripper hid a call. That
catches it for any spelling, cannot be inverted, and IS the measurement rather
than depending on one staying true.

THE READER HAD NEVER BEEN SHOWN THE PRODUCER'S OWN OUTPUT. Every log in both
harnesses was a literal, and the format string lives a third time in
pgc_summary. Three hand-written copies of one line: a wording drift in the
PRODUCER leaves both harnesses green while the reader answers "no" for every real
suite, which would redden the whole matrix on both majors having passed its own
tests. Both harnesses now run a real two-line suite and feed the reader its
actual stdout, with a reworded control so the arm can fail.

THE TWELVE, MEASURED. My prose said ten suites "never counted a check" and my
partition said twelve; both reviewers then compounded it, because a loose pattern
for sourcing lib.sh matches portlib.sh. Measured with a tight one: NONE of the
twelve sources test/lib.sh. Each defines its own check(), and ten keep no tally at
all. The number is no longer written in prose anywhere; the reconciliation prints
it at runtime.

THE OVERCOUNT THE PR OPENS BY DESCRIBING IS NOW ACTUALLY REPORTED. #922's own CI
showed "suites that ran: 242 of 251" still counting all twelve suites whose checks
the harness cannot see -- so the change described a fix it did not make. The
summary now breaks them out, from data already in hand at that point:

    suites that ran: 242 of 251 (skipped: 9, incomplete: 0)
    of those, N accounted for their checks and M did not

Also: an absent-file answer the case now handles with a loud default arm rather
than folding an unknown verdict into "does not declare"; a sentence on why the
log reader deliberately does NOT distinguish absent from negative while the
declaration reader does; a precise note that inputs == sum(buckets) cannot be
false on the DATA -- for sets the identity always holds, measured over 400 random
pairs -- and that what it guards is comm reading unsorted input; and ci.yml's
"three wall-clock suites", which is stale at four and which this change is the one
to falsify.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
…s it (#486)

#923's `suites (PG 17)` went red on a pytest name that EXISTS, while PG 18 passed
the same commit:

    FAIL  every test the document names exists in the corpus:
          got [[1: test_the_partition_over_the_registered_suites_adds_up]]

The cause is the shape selftest 080 already forbids. Its membership test was
`printf '%s\n' "$ondisk" | grep -qxF "$name"`, and the selftest runs under
`set -o pipefail`: grep -q exits the moment it matches, printf takes EPIPE, and
pipefail reports the pipeline failed though the name WAS present. The name is
then recorded absent.

WHY IT SURVIVED, AND WHY IT SURFACED NOW. Selftest 080's sweep is deliberately
non-recursive and never looked inside test/selftest/ -- the directory scoping was
never a decision, it fell out of writing "$TESTDIR"/*.sh, exactly as the bench/
hole did before it. Three fragments held the forbidden shape: 350's corpus
membership test, 300's directory coverage test, 340's Makefile sweep.

At corpus size the writer is small enough to win the race on an idle machine,
which is why it passed for so long. Measured, 170 names over 400 trials:

    printf | grep -qxF     idle: 0 false absences    under load: 6
    grep -cxF <<<          idle: 0                   under load: 0

A four-way CI matrix is the loaded case, and adding names to the corpus made the
window wider. This is the third appearance of this bug class here after #473 and
#476, and the second today.

All three sites now use grep -c on a here-string, and the sweep covers
test/selftest/ with its own coverage arm, because 080 already records that a
conditionally added glob narrows silently and a file-count premise cannot see it.

THE EXEMPTION IS DERIVED, NOT LISTED. 080's own control is inside a quoted
heredoc, and so is any deliberate demonstration of the shape; a line inside one is
text being written to a file, not a pipeline the suite runs. Comment lines are
excluded for the same reason -- the rule's explanation, and the note beside each
site fixed here, necessarily spell the shape out. A filename allowlist would have
to be maintained, and this rule exists because things that must be maintained are
not.

Two defects in the exemption itself, both found by running it rather than reading
it. It printed NR where it meant FNR, so from the second file onwards it reported
line numbers from a running total and no key matched -- the neighbouring question
answered plausibly, since single-file runs agree because NR == FNR there. And it
kept heredoc state across inputs, so it now resets per file. Its own arms pin
both: the sweep sees both lines of a probe, the exemption covers the one inside
the heredoc and not the one above it.

The "did not swallow the corpus" premise is a PROPORTION rather than a guessed
ceiling. The first version used a bare 2000 and went red at 2,502 heredoc lines in
a corpus that was entirely healthy -- a hand-written number failing the way
hand-written numbers fail here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK

@linuxhikerpm linuxhikerpm left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking: the reconciliation never receives the registered suite set, so a registered suite with no declaration and no accounting remains invisible and still passes. That is the live defect this PR says it fixes.

Reviewed exact head 44e2e9b95c6c2f0957361e5a60135ee9fd920aaf.

The matrix builds:

accounting.declared   suites whose source mentions a pgc_summary call
accounting.observed   suites whose log contains the accounting line
accounting.notdispatched

and calls:

pgc_reconcile_accounting "$declared" "$observed" "$notdispatched"

SUITES / the registered-name set is not an input. Inside the function, _inputs is the union of the declared and observed/not-dispatched files. Therefore a registered suite in neither side is absent from the universe being reconciled. declared == observed == empty is reported as complete symmetry.

Driven from the function on this head:

accounting reconciliation: inputs=0 | both=0, declared only=0, accounted only=0 | sum=0
empty_sets_rc=0

# create a separate registered set containing "registered_only", but the function
# has no argument through which it can observe that set
accounting reconciliation: inputs=0 | both=0, declared only=0, accounted only=0 | sum=0
registered_but_unpassed_rc=0

This is not hypothetical. The PR body identifies ten registered suites that exit 0 without calling pgc_summary. They remain in exactly that no/no bucket, pgc_classify_suite_rc still maps their rc=0 to PASS, and the new reconciliation still excludes them from _inputs. The selftest even requires the declaration reader's no bucket to be occupied, but never requires those names to be accounted by another mechanism.

The printed inputs == sum(buckets) identity is independent only inside the reduced declared/observed universe. It cannot detect registered names omitted from both, so it does not reconcile “registered suites against accounted ones” as the PR title claims.

Please pass a fourth REGISTERED set into pgc_reconcile_accounting and explicitly fail names in:

registered - (observed ∪ notdispatched)

A source declaration can remain a useful second observer for “promised accounting but died before summary,” but it cannot define the population. If the twelve current non-declaring suites are intentionally exempt, they need a separately derived, runtime-observable accounting mechanism; treating absence of a declaration as absence from the population preserves the overcount.

Add the direct red arm: registered={alpha}, declared/observed/notdispatched empty must fail and name alpha. Also retain the opposite-direction and clash arms already present.

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Checked the new head 324357c after my review landed. The blocker remains.

The latest commits improve absent-file detection and now print:

of those, N accounted for their checks and M did not

but pgc_reconcile_accounting still takes only (declared, observed, notdispatched), _inputs is still declared ∪ observed, and pgc_tally_suite still increments suites_ran for every rc=0 PASS whether it accounted or not. The registered set is still not reconciled and the non-accounting bucket is reporting only.

This is exactly the originating issue's stated limit:

Any reconciliation is a lie for those twelve until they are brought in or exempted with a premise that fails when the list grows. An exemption list that can silently lengthen is the same defect one level up.

Using “does not declare pgc_summary” as the exemption property makes that exempt set lengthen automatically when a suite accidentally loses its summary—the failure #916 says must be caught. The current real-population arm explicitly refuses to pin the no bucket, so that growth remains silent.

The required red case is unchanged: REGISTERED {alpha}, with declared/observed/notdispatched empty, must fail and name alpha. On 324357c there is still no function argument through which alpha can be observed, so the function returns 0.

jdatcmd and others added 2 commits September 9, 2026 21:22
…d printf (#486)

Reported by OffgridwithJD against the previous commit: widening the DIRECTORIES
left the PRODUCER scoped to echo and printf, so every `<command> | grep -q` was
still unswept. Twenty-six sites, two of them in lib.sh and shared by every suite
that asks whether a plan is a columnar scan.

THE SCOPING REVERSAL, AND WHY. The rule's own stated principle is a reader that
exits early AND whose EXIT STATUS is the answer being read. That is
producer-independent: the writer takes EPIPE whether it is a builtin, a psql, an
ldd or an ss. Scoping to echo and printf was a narrower implementation than the
principle, justified in the file by "a pipeline out of psql or a file is a
different question" -- which is true of `| head -1` used as TEXT and false of
`| grep -q` used as a VERDICT.

THE WORST SITE IS VACUITY, NOT A FALSE RED. native_vecskip.sh's "premise: and it
is not the scalar scan" WANTS "no", so a spurious EPIPE answer makes that premise
pass for the wrong reason. It is fixed first for that reason.

NOT CLAIMED TO BE LYING TODAY. Measured, 200 trials per size on a loaded box,
match always on line one so the answer is knowably yes:

    1,892 bytes (EXPLAIN-sized)    0/200 wrong
    8,893 bytes                    1/200   <- first observed lie
   66,894 bytes                   21/40
  288,894 bytes                   40/40
  control, match on the LAST line so grep reads to EOF: 0/200 at every size

Latent, with no floor in the mechanism -- the probability rises with size rather
than crossing a threshold, which refuted a clean pipe-capacity hypothesis.
Reasoning about "small enough" is how #473, #476 and selftest 350 each survived,
so the rule sweeps rather than reasons.

`||` IS NOT A PIPE, and the first version of the widened pattern thought it was:
`[ "$rc" = 124 ] || grep -q PAT <<<"$out"` is a fallback branch reading a
here-string, with no writer process and so no EPIPE, and both fuzz suites were
flagged for it. The leading [^|] excludes it, with an arm pinning that.

My own deliberate twin in selftest 390 moved into a quoted heredoc, so the
widened sweep exempts it by the property already built rather than by a line
number -- a filename list is the thing this rule exists to avoid.

THE MATRIX IS THE VERIFICATION HERE. lib.sh's pgc_is_columnar_scan and
pgc_uses_row_fetch are shared by many suites, and eighteen suite files changed.
The selftest cannot exercise them; the two `suites (PG N)` jobs can. Local:
selftest exit 0, 553 checks, 0 failures, shellcheck rc=0 over test/, selftest/
and bench/.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
(cherry picked from commit 0d49e20)
…nchor (#916)

Both reported by OffgridwithJD against 0ce14d7, and both are the shapes this
change exists to refuse, committed inside it.

THE PARTITION ARM WAS AN IDENTITY. `_reg` was incremented in the same loop body
as the three buckets, so their sum equalled it for ANY reader. Proven rather than
argued: it passes with an always-yes reader and with an always-no reader alike,
and adding the absent bucket did not change that. A total derived from the loop
that produces the buckets cannot fail, which is the note I had just re-worded two
functions away for the same reason.

The population is now counted by a SECOND ROUTE -- the runner's own --list-suites
-- so the arm fails when the classification does not see every registered suite:
a future `continue`, a read that drops a line, a list that changes between the two
reads.

And it now says what it is. It is a COVERAGE check, not a check on the reader's
correctness. The two arms below it, which require both buckets to be occupied, are
what catch a reader that answers the same way for everything. Overrating it is how
it survived as an identity.

NOTHING EXERCISED THE ^ ANCHOR. pgc_log_shows_accounting's comment claims
"anchored and fully shaped, so the word appearing in a suite's own prose cannot
satisfy it", and the prose fixture is refused by the regex SHAPE rather than by
the anchor -- so removing ^ from the reader left every arm in both harnesses
green. The indented fixtures added earlier are for the DECLARATION reader and do
not reach this one.

The distinguishing input is a well-formed accounting line that does not start its
line. Measured against a twin with the anchor removed, mutation asserted applied:

    indented line, real reader (anchored)     no
    indented line, twin reader (unanchored)   yes
    control, line-start, real reader          yes

Inert on real data -- 0 non-line-start occurrences across 246 PG17 logs and 244
PG18 -- so this closes a coverage gap rather than a live defect.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK

@OffgridwithJD OffgridwithJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving at 09ec8b71, 12/12 green. Everything load-bearing below I ran myself; where a number is yours I say so.

The claim this PR rests on is verified over the real population

Not from the summary — from the job logs. ci.yml's suites step runs PGC_SKIP_TIMING=1 PGC_JOBS=4 PGC_REQUIRE_ISOLATION=1 bash test/run_all_versions.sh, so the reconciliation executes against 251 real logs, and it printed the same line on both majors:

accounting reconciliation: inputs=239 | both=239, declared only=0, accounted only=0 | sum=239

An agent of mine also ran three full PG18 matrices locally, including the arm CI never runs (PGC_SKIP_TIMING unset, so the notdispatched term is empty), with zero false positives in all three, and snapshotted $builddir/accounting.notdispatched before teardown to confirm the term holds driver-written names rather than inferred ones.

What I found, and the proof each fix bites

The corpus arm in 65bf569 was inverted. [^[:space:]]# requires a non-space before the hash — a hash inside a word, which the new stripper handles correctly — so it was blind to the hazardous shape and fired on the safe one:

psql -c "SELECT 1 # note"; pgc_summary   reader: no    arm: not flagged   <- the hazard
X=a#b; pgc_summary                        reader: yes   arm: FLAGGED       <- the safe shape
pgc_summary                               reader: yes   arm: not flagged   <- control

Replaced with input-versus-output counting, which is the measurement rather than a second expression of it. Worth recording how the labour now splits, because it is not obvious: reverting the stripper to the blind s/#.*$// changes 0 of 251 suites' answers, so the corpus arm is a future tripwire; what pins the implementation is the hashinword fixture, the only one of four where the two strippers disagree. Both arms are needed and neither is redundant.

The log reader had never been shown the output of the thing that produces it. Both harnesses hand-wrote the format while it lives in a third place, lib.sh:1507. The new test_the_reader_accepts_the_line_the_producer_actually_emits closes that, and it bites: mutating the producer's format string (anchor count 1 before, 0 after, restored byte-exact) gives 1 failed, 8 passed, reddening exactly that arm.

An absent suite file read as exempt, so a registered suite whose .sh vanished reconciled clean — inside the check whose subject is suites going missing from the accounting. Now a third bucket, and the real-population block carries three rather than folding absent back into exempt.

ci.yml:475 said "the three wall-clock suites". is_timing_suite has four arms and is byte-identical to main (md5 e484be8888184c8ae6aaf20721b3caae), and the CI log shows four SKIP … (PGC_SKIP_TIMING) lines. Fixed here, which is right: this is the PR that made the contradiction visible.

The partition arm could not fail. _reg was incremented in the same loop body as the buckets, so the sum equalled it for any reader — it passed with an always-yes reader and an always-no reader alike. _reg now comes from a second route, and it reddens properly: dropping one suite from the classification loop gives sum=250 against _reg=251. The arm also now says what it is — a coverage check, not a check on the reader's correctness — which matters, because overrating it is how it survived as an identity.

The ^ anchor was untested. The only prose fixture is rejected by the regex shape, so removing ^ left every arm green. The new indented-line fixture reddens against an unanchored twin: anchored no, unanchored yes, control yes.

On the number

"Ten registered suites exit 0 having never counted a check" is correct in the runner, TESTS.md, the pytest docstring and the PR body, and I had it wrong when I first said otherwise. Measured: none of the twelve sources test/lib.sh, each defines its own check(), and exactly two maintain a tally — bench_guards (its own PGC_CHECKS, printed at its line 373) and docs_style. Ten keep no counter at all. Two true numbers for two properties; the one site that paired ten with "never called pgc_summary" is fixed.

The twelve are audit bench_guards concurrency docs_style phase2 phase3 phase4 phase5 phase6 smoke unique_conc update_conc. They sit outside the reconciliation by construction, and the summary line now reports how many accounted rather than leaving them silently inside "ran" — which closes the overcount the first paragraph opens with, rather than describing it.

Two things I raised and killed myself

local _plan="$(psql …)" takes psql's exit status, so under set -e it could abort where the old shape answered "no". It does not bite: the eight set -euo suites make zero calls to either rewritten helper, all twelve callers are set -uo pipefail, and forcing set -euo pipefail with the substitution inside a check argument still answers "no" and runs to the end.

And inputs == sum(buckets) cannot fail on the data — for sets those are equal identically, and 400 random set pairs fired it zero times. That is not a defect, and the file now says what it is: a comm-usage tripwire, which the unsorted-twin mutation exercises correctly.

What I did not verify

The eighteen suite files' own plan-shape patterns against a live cluster, beyond what the two suites jobs exercise. I read both rewritten shared helpers line by line and they are behaviourally identical to the originals on every input I could construct — pattern present, absent, empty output from a failed psql (both answer "no"), multiple matches — the only difference being that the producer now runs to completion.

The EPIPE widening, since it rides here

Scoping the rule to echo/printf producers left <command> | grep -q unswept, including lib.sh's own two plan-shape helpers. Measured, 200 trials per size, match always on line one:

producer bytes wrong
292 / 692 / 1,892 / 3,893 0/200
8,893 1/200
66,894 21/40
288,894 40/40

Control (match on the last line, so grep reads to EOF) is 0/200 at every size; both fixes are 0/40 at the worst size. So the sites were latent rather than lying — and with no floor in the mechanism, which is the argument for sweeping rather than reasoning about "small enough". native_vecskip.sh:105 was the one that mattered most, because that arm wants the answer "no", so the race made it pass for the wrong reason.

Merge order, for whoever lands these: #921 first. Its deferred-psycopg conftest is the precondition for this PR's new pytest file to run in the gate at all, so the reverse order ships those eight tests ungated for as long as #921 is open.

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

@linuxhikerpm's blocking point is correct, it still holds at 09ec8b71, and I do not want my approval read as overriding it. I verified the structure rather than reasoning about it:

pgc_reconcile_accounting() {   # pgc_reconcile_accounting DECLARED OBSERVED [NOTDISPATCHED]
...
1138:  if ! pgc_reconcile_accounting "$_acc_declared" "$_acc_observed" "$_acc_notdisp"; then

Three files. SUITES is not among them, so registered − (declared ∪ observed ∪ notdispatched) is outside the universe being reconciled, and declared == observed == ∅ reads as complete symmetry. I reached the same conclusion independently from the other end — this PR's own CI run prints suites that ran: 242 of 251 (skipped: 9, incomplete: 0) while all twelve non-declaring suites are classified PASS — so the overcount the opening paragraph names is still there, and the reconciliation is silent about it in both directions.

Where I land, given I approved: what I approved is what the PR does — a drift detector for the 239, which is real and well built, plus honest reporting. What the title claims is wider than that. So one of two things should happen, and I do not think it matters much which:

  • pass the registered set in as a fourth argument, or
  • retitle to what it reconciles — declared against accounted.

The constraint that makes the first option non-trivial, and a shape that fits it

Failing on registered − (observed ∪ notdispatched) reddens CI today, because that set is exactly the twelve: audit bench_guards concurrency docs_style phase2 phase3 phase4 phase5 phase6 smoke unique_conc update_conc. They are not a hypothetical bucket; they are a dozen suites that each define their own check() and never call pgc_summary. Measured: none of the twelve sources test/lib.sh, and exactly two of them (bench_guards, docs_style) maintain a tally at all.

So I would land it in this shape:

  1. Pass the registered set in now, and report the bucket — the function can then see the population, which is the structural fix, and the summary line already has the partition in hand at the point it prints.
  2. Add @linuxhikerpm's red arm exactly as asked — registered={alpha}, declared/observed/notdispatched empty, must fail and name alpha — driven against fixtures, which is where every other arm in selftest 390 lives. That proves the mechanism fails when the population is unaccounted, without the live run reddening.
  3. Make it fail on the live population only once those twelve have a runtime-observable accounting mechanism. That is Phase 3 of #858: check results are prose, so a named check cannot be cited mechanically #917's subject, and test: count a check and record it in one operation (#917) #923 already builds it: one pgc_record per check, so checks run: N and N record lines are the same increment seen twice. A suite that adopts lib.sh's accounting leaves the bucket on its own, which is the same derived-membership argument this PR makes everywhere else.

That ordering also keeps the rule from being switched off, which is the failure mode this whole family of issues exists to prevent: a check that reddens twelve times on every CI run from the day it lands is a check someone disables in a week.

One correction to the review above, which strengthens rather than weakens it

The review says "the PR body identifies ten registered suites". The count is twelve for that predicate and ten for a different one, and both numbers are true: ten registered suites never count a check, twelve never call pgc_summary. I published the wrong version of this myself earlier and had to retract it. It does not affect the argument — the no/no bucket is the twelve either way — but the distinction matters for whichever fix lands, because the population that needs a mechanism is the twelve, not the ten.

My approval stands for the drift detector and for the six findings that were fixed and which I verified bite. I am not asking for it to be counted against the structural point.

OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 10, 2026
The gate job's file list came from NO_CLUSTER, a hand-written list, and nothing
decided whether it was RIGHT. The only arm checked that the names it held exist,
and at_least(len(NO_CLUSTER), 4) is satisfied by any list of five. So a new
database-free test file was silently skipped by the job and nothing went red --
a coverage hole in the mechanism that exists to give those tests coverage.

Membership is now DECIDED, from a property of each file, and reconciled against
the declaration in BOTH directions. The property is read with ast, not a line
regex: this corpus builds tests as strings for pytester, so a file that merely
MENTIONS the driver in prose must not count, and one that reaches a cluster only
through a fixture must. The cluster fixtures are read off conftest.py rather than
named here, so adding one does not need a second edit.

    NO_CLUSTER missing a database-free file   [1: undeclared:test_ordered.py]
    NO_CLUSTER claiming a cluster test        [1: needs-a-cluster:test_connection.py]

Both reddened; mutations count-asserted and restored byte-exact. The message
names the offender and which way the disagreement goes, because "the lists
differ" is not something a reader can act on.

AND NO COUNT IS WRITTEN ANYWHERE. Six sentences stated "61 of 142"; the corpus
is now 168 tests, and the ci.yml comment said 152 against a corpus of 154, so it
was wrong the day it was written. The gate's step prints how many files it ran
and pytest prints how many tests passed, which is commandprompt#908's rule: a derived value
that a human maintains is a defect, and the corpus gate provably cannot police
prose -- its totals guard matches only the bold fixed-form line commandprompt#908 removed.

Three smaller things, each found by reading this change rather than the tree:

A table-of-contents link must RESOLVE, not merely name a file. TESTS.md gained
an entry whose anchor stripped the underscores out of test_harness_deps.py, so
the link went nowhere while both existing arms passed -- they sweep for NAMES,
and a broken link still contains the name it points at. The rule is GitHub's and
mechanical, it has ONE definition in this file, and the sweep carries a coverage
premise because without nullglob an unmatched glob stays literal and a loop that
runs no checks reports every check it did run as passing.

Two `printf ... | grep -qxF` pipelines are gone, one of them written by this
change. Under this suite's pipefail grep -q exits on the first match, printf
takes EPIPE, and the pipeline reports failure though the pattern WAS present --
so a name that matched is counted absent. Measured at 10 spurious absences in 40
runs under load; 40 runs of the rewritten sweep over the largest document give
one distinct answer. Selftest 080 states this rule and its sweep has never
entered test/selftest/, which commandprompt#486 is fixing separately.

    harness_selftest  465 passed + 0 failed + 0 unrunnable = 465   PASSED
    pytest corpus     168 passed
    shellcheck -S error -s bash  clean

One consequence to merge in order: with this landing first, commandprompt#922 adding a
database-free test file will REDDEN this arm until that file is declared. That is
the hole closing, not a regression -- before this change the file would have been
skipped in silence.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a
… fails (#916)

Blocking review by @linuxhikerpm, and the finding is structural and correct.

pgc_reconcile_accounting takes the DECLARED set and the OBSERVED set. Both are
derived from the suites themselves, so a registered suite in NEITHER is outside
the universe it reconciles. Driven from the function, on the head under review:

    accounting reconciliation: inputs=0 | both=0, declared only=0, accounted only=0 | sum=0
    rc=0

with the registered set holding a name the function has no argument to see. As
they put it: treating absence of a declaration as absence from the population
preserves the overcount this change is named for.

THE POPULATION IS NOW ITS OWN CHECK, and the registered set is its first input.
Every registered suite lands in exactly one of four buckets:

    accounted        its log shows it counted its checks
    not dispatched   the driver recorded that it never ran it
    known debt       named in a tracked debt file
    unaccounted      none of the above -- FAILS, by name

ACCOUNTED TAKES EITHER MECHANISM, both runtime-observable. pgc_summary's
accounting line covers 239 suites. bench_guards and docs_style keep private
counters and print their own `checks run:` without ever sourcing lib.sh, so a
reader that knew only the first would call them unaccounted, which is false.
Measured over all twelve non-declaring suites: exactly two print a runtime count,
ten print none -- which is where the ten and the twelve come from, and why they
are different numbers.

Both mechanisms are derived rather than declared, so a suite that adopts either
leaves the debt bucket on its own. That is the property that stops the debt file
becoming a permission slip.

THE DEBT FILE IS DEBT. test/suites_without_accounting.txt names the ten suites
that count nothing at runtime, generated from a measurement rather than typed. It
is tracked, so adding a name is a diff a reviewer sees -- which is why it is a
file and not a number in the environment, per #858's own constraint. A name that
starts accounting, or stops being registered, is REPORTED rather than fatal: a
gate that reddens the moment someone fixes something teaches people not to.

Failing the live population outright would redden CI ten times from the day it
lands, which is the failure mode this family exists to prevent. Recording those
ten by name makes an eleventh fail while the ten are excused, which is the
difference between a burn-down and an exemption.

Every one of the three ways out is asserted to actually let a suite out, or
"unaccounted" would be a name for "always fails" and only the debt file would be
doing any work. And inputs == sum(buckets) over the registered population CAN be
false on the data, unlike the symmetry check's identity: a name can fall outside
all four buckets.

Both harnesses, and the arm @linuxhikerpm asked for verbatim: registered={alpha},
everything else empty, must fail and name alpha.

One of my own arms was wrong on the way: it asserted the string
"accounting.registered" appeared twice, which is a fact about how many times a
variable is spelled. It now asserts the property -- that the registered file is
written from the SUITES array itself.

Evidence: selftest exit 0, 538 checks, 0 failures; 14 pytest; shellcheck rc=0
across test/, selftest/ and bench/; docs_style PASSED. The live population is the
matrix's job and it has not run yet.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
@jdatcmd

jdatcmd commented Sep 10, 2026

Copy link
Copy Markdown
Collaborator Author

Your blocker is correct, it is structural, and it is fixed in 4eca4d2. I drove your reproduction myself before accepting it rather than reading the argument:

  registered set holds: registered_only
  accounting reconciliation: inputs=0 | both=0, declared only=0, accounted only=0 | sum=0
  rc=0                                   <- unchanged, because SUITES is not an argument

  parameters the function actually reads: $1 $2 (and ${3:-})

You put it better than I would have: treating absence of a declaration as absence from the population preserves the overcount. That is exactly what the title claimed to fix and the function could not see.

What changed

pgc_reconcile_population REGISTERED ACCOUNTED NOTDISPATCHED DEBT takes the registered set as its first input. Every registered suite lands in exactly one of four buckets, and the last one fails by name:

bucket meaning
accounted its log shows it counted its checks
not dispatched the driver recorded that it never ran it
known debt named in a tracked debt file
unaccounted none of the above — fails

Your red arm is in verbatim: registered={alpha}, everything else empty, must fail and name alpha. Both harnesses. The opposite-direction and clash arms on pgc_reconcile_accounting are retained — it is still a useful second observer for "promised accounting but died before summary", it just no longer pretends to define the population.

The one place I did not do exactly what you asked, and why

You asked for failure on registered − (observed ∪ notdispatched). That set is exactly the twelve, so the check would redden CI twelve times on the day it lands — and a check that reddens on every run from day one is one somebody switches off in a week, which is the failure mode this whole family of issues exists to prevent.

So the twelve are recorded by name in test/suites_without_accounting.txt, tracked, so adding a name is a diff a reviewer sees. That is #858's own stated constraint for the budget, one level up. The difference between that and an exemption is that an eleventh unaccounted suite fails while the ten are excused — asserted, not asserted-about.

Your "separately derived, runtime-observable accounting mechanism" already exists for two of them

I measured all twelve rather than assuming. Exactly two print a runtime count of their own checks:

  bench_guards   prints "checks run:"=2   keeps a counter=2
  docs_style     prints "checks run:"=1   keeps a counter=1
  the other ten  prints "checks run:"=0   keeps a counter=0

So pgc_log_shows_any_accounting takes either mechanism — pgc_summary's accounting line, or a suite's own checks run: line. Both runtime-observable, both derived rather than declared, so a suite that adopts either leaves the debt bucket on its own. That is what keeps the debt file from becoming a permission slip, and it is why the debt is ten and not twelve.

It also settles a number that has been wrong in three places: "ten registered suites exit 0 having never counted a check" and "twelve exit 0 without ever calling pgc_summary" are both true, of different properties.

Also fixed here

Debt that is no longer debt — a suite that now accounts, or one no longer registered — is reported, so a burn-down cannot stall silently. Reported rather than fatal, deliberately: a gate that reddens the moment someone fixes something teaches people not to fix things.

Each of the three ways out is asserted to actually let a suite out, or "unaccounted" would be a name for "always fails" and only the debt file would be doing any work. And inputs == sum(buckets) over the registered population can be false on the data, unlike the symmetry check's identity — a name can genuinely fall outside all four buckets.

Evidence

selftest    exit=0  FAILs=0   538 checks
pytest      14 passed
shellcheck  -S error over test/, selftest/ and bench/   rc=0
docs_style  PASSED

The live population is the matrix's job. It is running now, and I will post the reconciliation lines from it rather than predict them.

Thank you for the block — it was right, and the reproduction driven from the function is what made it impossible to argue with.

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

@linuxhikerpm's blocking point is closed, and I checked it by driving the new function rather than reading it. One new finding, small, and it is a repeat of a class this PR corrected two functions away.

The block is closed

Their red arm, verbatim — registered={alpha}, accounted/notdispatched/debt all empty:

rc=1
    registered but accounted by nothing: alpha
  population reconciliation: registered=1 | accounted=0, not dispatched=0, known debt=0, unaccounted=1 | sum=1

And each of the three ways out actually lets a suite out, which is the part that stops "unaccounted" from being a name for "always fails": via accounted rc=0, via notdispatched rc=0, via debt rc=0.

The divergence from my proposal is correct, and I measured it independently

I had suggested deferring the live failure until the twelve gained a runtime-observable mechanism. Two of them already have one, so the debt is ten, and the file lists exactly those ten with bench_guards and docs_style excluded. Verified rather than taken: both print their own checks run: line from a private counter, and running docs_style.sh for real gives rc=0 with one line matching ^checks run: [0-9]+$, where the new reader answers yes and the accounting-line-only reader would have answered no.

That also retires the number that was wrong in three places yesterday. "Ten never counted a check" and "twelve never called pgc_summary" are both true of different properties, and nothing now has to choose between them: the reader takes either mechanism, so the population question and the predicate question stop being entangled.

Treating the file as debt that may only shrink, generated from the measurement, and reporting stale entries rather than failing on them, is the right shape — a gate that reddens when someone fixes something teaches people not to fix things.

One finding: registered == sum(buckets) cannot fail on data either

The buckets are a strict partition of the registered set by successive subtraction:

_t1    = registered \ accounted          _nacc  = |registered ∩ accounted|
_t2    = _t1        \ notdispatched      _nnd   = |_t1 ∩ notdispatched|
_unacc = _t2        \ debt               _ndebt = |_t2 ∩ debt|
                                         _nunacc = |_unacc|

so _nacc + _nnd + _ndebt + _nunacc equals _nreg identically. Measured, 400 random four-set inputs: "the population does not add up" fired 0 times, while the real bucket checks returned non-zero on 260 of them — so the function is doing real work and this line is not the part doing it.

This is the same class as the inputs == sum(buckets) line you re-worded earlier in this PR, and the words you used there fit here: at best it is a comm-usage tripwire, not a reconciliation of the data. Either say that, or give it teeth by counting one side through a route that is not the subtraction chain — which is exactly the fix applied to _reg in selftest 390's partition arm, and it is worth noting that arm and this one now differ in which direction is derived.

Not blocking. My approval from 09ec8b71 stands on the record but no longer describes the head, so I will re-approve on 4eca4d22 once its two pending suites jobs land — the population reconciliation running over the real 251 suites is the arm I most want to see green, for the same reason the first one was.

@OffgridwithJD OffgridwithJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-approving at 4eca4d22, 12/12 green, so the approval describes the head rather than a tree that has moved under it. My earlier approval at 09ec8b71 stands on the record; this replaces it in substance.

What changed since then is @linuxhikerpm's blocking point, and I verified the fix by driving the new function rather than reading it:

registered={alpha}, accounted/notdispatched/debt empty:
  rc=1
      registered but accounted by nothing: alpha
    population reconciliation: registered=1 | accounted=0, not dispatched=0, known debt=0, unaccounted=1 | sum=1

each way out, separately:   accounted rc=0   notdispatched rc=0   debt rc=0

The last line matters as much as the first: if the ways out did not work, "unaccounted" would be a name for "always fails" and only the debt file would be doing anything.

I also checked the divergence from what I had proposed, rather than taking it. Two of the twelve already have a runtime-observable mechanism, so the debt is ten: docs_style.sh run for real gives rc=0 with one line matching ^checks run: [0-9]+$, the new reader answers yes, and the accounting-line-only reader answers no. The debt file lists exactly those ten with bench_guards and docs_style excluded. That is a better answer than mine, and it retires the ten-versus-twelve confusion by making the reader accept either mechanism instead of making the population question choose a predicate.

My one open finding — registered == sum(buckets) is an identity by successive subtraction, 0 fires in 400 random four-set inputs while the real bucket checks fired on 260 — is in the comment above. It is not a defect and it does not block: the three bucket checks do the work, and the line is a comm-usage tripwire at best. It is worth one sentence saying so, because the same misreading two functions away is what you had just corrected.

Merge order unchanged and now unblocked: #921 is 13/13 green at 2ad25372. Land it first — and its merge has one coupling, stated on that PR: with #921 in, this PR's new database-free test file reddens the derived-membership arm until test_suite_accounting.py is added to NO_CLUSTER, so that one line belongs in this merge rather than in a follow-up.

Reported by OffgridwithJD, and it is the same class I re-worded two functions
away in this very PR: the four buckets are built by successive subtraction FROM
the registered set, so their sum equals it identically. Measured, 400 random
four-set inputs: the identity fired 0 times while the real bucket findings fired
on 353 of them.

The line stays -- the house rule asks for inputs == sum(buckets) printed beside
any list-derived claim -- but the comment now says what it can catch, because the
next reader will otherwise go looking for a data case that does not exist. What
it guards is comm being fed unsorted input, which produces buckets that are not a
partition at all.

TESTS.md and the pytest docstring said the opposite -- that this identity CAN be
false on the data, unlike the symmetry check's. That was my claim and it was
wrong; both are corrected, and both now point at the arms that carry the weight:
the ones on the unaccounted bucket.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK

@linuxhikerpm linuxhikerpm left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed exact head 9dfb63ef0067924144fade55963efc92316c21dd. My blocker is resolved.

The required empty-accounting control now fails with rc=1 and names alpha when registered={alpha} and accounted/not-dispatched/debt are all empty. Each legitimate escape route—accounted, not dispatched, and named debt—returns rc=0. The runner passes the registered suite set and propagates reconciliation failure.

Independent focused evidence in cursor-2604: 14/14 pytest tests pass serially and under -n 4; the real partition is 251 = 239 summary-accounted + 2 private-accounted + 10 named debt. The full selftest reached 537/538, with the only failure an unrelated installed-binary hash mismatch; every new accounting assertion passed. No remaining actionable finding in this delta.

@jdatcmd
jdatcmd merged commit f0f1f40 into main Sep 10, 2026
12 checks passed
OffgridwithJD pushed a commit to OffgridwithJD/pgcolumnar that referenced this pull request Sep 10, 2026
…ompt#922's new file

commandprompt#922 merged first, so main now carries test/pytest/test_suite_accounting.py, which
needs no database. This branch's membership arm asserts set equality between the
declared NO_CLUSTER list and the files an ast property says are database-free, so
an undeclared database-free file reddens it. It did, before the entry was added:

    FAILED test_the_declaration_is_exactly_the_database_free_half
    FAILED test_the_gate_runs_the_membership_decision_rather_than_only_this_file
    2 failed, 18 passed

With "test_suite_accounting.py" declared: 20 passed, the partition is 7
database-free against 6 cluster-bound, and membership_report() returns []. That is
the coupling working rather than a cost: before this branch the file would have
been skipped by the guards job in silence.

TWO CONFLICTS, both resolved deliberately.

test/selftest/350 was COMMENT-ONLY. commandprompt#922 and this branch independently fixed
_dcv_absent's EPIPE bug and made the SAME fix -- grep -cxF on a here-string --
so the code line is identical on both sides and sits outside the conflict. That
was asserted rather than eyeballed: zero non-comment lines on either side, which
is what rules out a careless resolution restoring `printf | grep -qxF`. commandprompt#922's
comment is the base because it carries the commandprompt#923 provenance, and this branch's
measurement is folded in as a second data point: 6 false absences in 400 trials at
170 names under synthetic load, and 10 in 40 in isolation. They bracket the rate
rather than disagreeing, so the comment now says it is load- AND size-dependent.

test/pytest/TESTS.md was the table of contents and the section bodies. commandprompt#922's
section 14 keeps 14 and this branch's becomes 15, with Adding a test, What this
corpus does NOT yet refuse, and Traps this corpus records shifting to 16, 17 and
18. Checked structurally rather than by reading: 18 headings, 18 contents entries,
contiguous 1..18, and every anchor matches its heading under GitHub's own rule --
which this branch's own sweep is what enforces.

Gate on the merged tree, /usr/local/pg17a, PGC_SKIP_BUILD unset:

    harness_selftest   561 passed + 0 failed + 0 unrunnable = 561, PASSED
    pytest corpus      182 passed
    the guards subset  7 files, 127 passed, with psycopg shimmed to raise on import

The last line is the job this branch adds, run under its own condition. Its control
is that the cluster-bound files still FAIL there: test_connection.py 8 errors and
test_hilbert_locality.py 18 errors under the same shim. Without that control,
"the guards passed" is equally satisfied by a harness that reaches no database at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Uf6UoeBRZYLQZa4KxNiw8a

@OffgridwithJD OffgridwithJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking at exact head 9dfb63ef0067924144fade55963efc92316c21dd: adversarial mutations found multiple fail-open paths that supersede my approval.

  1. The debt allowlist can grow silently. Adding an unaccounted alpha suite to suites_without_accounting.txt changes reconciliation from rc=1 to rc=0; stale debt also returns rc=0. “May only shrink” is currently commentary, not an enforced property. Compare against a tracked baseline/derivation so additions fail and removals remain allowed.

  2. Accounting is forgeable with a zero marker. A suite log containing only checks run: 0 is classified as accounted and population reconciliation returns rc=0. Require evidence of a real accounting mechanism and a positive/reconciled outcome where appropriate; a hand-written zero line must not exempt a registered suite.

  3. Missing or unreadable registered input fails open. Suppressed sort/read errors yield registered=0 ... sum=0, rc=0. The runner itself does not use set -e, so artifact/write failures can reach this path. Validate every input before set operations and return nonzero on missing, unreadable, unsorted-generation, or failed command output.

  4. Final reporting disagrees with reconciliation. The real partition is 239 pgc_summary + 2 private-counter suites + 10 debt, but the final report uses only the summary set and says 12 unaccounted. Report the same accounted universe the gate actually accepts.

Focused controls remain green: 14 pytest tests and 538 shell selftests pass, which confirms these failure modes are absent from the current tests rather than ordinary breakage.

Stack blocker: current #921 conflicts in TESTS.md/selftest 350, and its membership decider reports test_suite_accounting.py undeclared. The merged stack must reconcile that database-free test into the CI declaration.

@linuxhikerpm linuxhikerpm left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Blocking at exact head 9dfb63ef0067924144fade55963efc92316c21dd: adversarial mutations found multiple fail-open paths that supersede my approval.

  1. The debt allowlist can grow silently. Adding an unaccounted alpha suite to suites_without_accounting.txt changes reconciliation from rc=1 to rc=0; stale debt also returns rc=0. “May only shrink” is currently commentary, not an enforced property. Compare against a tracked baseline/derivation so additions fail and removals remain allowed.

  2. Accounting is forgeable with a zero marker. A suite log containing only checks run: 0 is classified as accounted and population reconciliation returns rc=0. Require evidence of a real accounting mechanism and a positive/reconciled outcome where appropriate; a hand-written zero line must not exempt a registered suite.

  3. Missing or unreadable registered input fails open. Suppressed sort/read errors yield registered=0 ... sum=0, rc=0. The runner itself does not use set -e, so artifact/write failures can reach this path. Validate every input before set operations and return nonzero on missing, unreadable, unsorted-generation, or failed command output.

  4. Final reporting disagrees with reconciliation. The real partition is 239 pgc_summary + 2 private-counter suites + 10 debt, but the final report uses only the summary set and says 12 unaccounted. Report the same accounted universe the gate actually accepts.

Focused controls remain green: 14 pytest tests and 538 shell selftests pass, confirming these failure modes are absent from the current tests rather than ordinary breakage.

Stack blocker: current #921 conflicts in TESTS.md/selftest 350, and its membership decider reports test_suite_accounting.py undeclared. The merged stack must reconcile that database-free test into the CI declaration.

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Administrative correction: the changes-requested review immediately above was posted under this account because another process changed the shared gh credential between identity verification and submission. It should be disregarded as an OffgridwithJD review. The same evidence has now been submitted under the intended reviewer account, linuxhikerpm. No technical conclusion changed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Phase 2 of #858: the bash runner does not reconcile registered against ran

3 participants